Email verification #4268
Email verification #4268
Conversation
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
| @@ -872,42 +883,42 @@ def dormant_user_ids(num_days_since_activity=5) | |||
| end | |||
| end | |||
|
|
|||
| describe "#send_welcome_email after_create callback" do | |||
| let(:user) { build(:user) } | |||
| describe "#send_welcome_email on confirm" do | |||
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| let(:user) { build(:user, confirmed_at: nil, confirmation_sent_at: nil) } | ||
|
|
||
| it 'sends a confirmation email' do | ||
| expect_any_instance_of(User).to receive(:send_confirmation_instructions).once |
There was a problem hiding this comment.
RSpec/AnyInstance: Avoid stubbing using expect_any_instance_of.
RSpec/DescribedClass: Use described_class instead of User.
| @@ -313,8 +313,8 @@ | |||
| expect(User.where(login: user_attributes[:login])).to_not exist | |||
| end | |||
|
|
|||
| it "should not queue a welcome worker to send an email" do | |||
| expect(UserWelcomeMailerWorker).to_not receive(:perform_async) | |||
| it "should not send a confirmation instructions email" do | |||
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
RSpec/ExampleWording: Do not use should when describing your tests.
| it "should queue a welcome worker to send an email" do | ||
| expect(UserWelcomeMailerWorker).to receive(:perform_async) | ||
| it "should send a confirmation instructions email" do | ||
| expect_any_instance_of(User).to receive(:send_confirmation_instructions).once |
There was a problem hiding this comment.
RSpec/AnyInstance: Avoid stubbing using expect_any_instance_of.
| @@ -279,8 +279,8 @@ | |||
| post :create, params: { user: user_attributes } | |||
| end | |||
|
|
|||
| it "should queue a welcome worker to send an email" do | |||
| expect(UserWelcomeMailerWorker).to receive(:perform_async) | |||
| it "should send a confirmation instructions email" do | |||
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
RSpec/ExampleWording: Do not use should when describing your tests.
| it "should not queue a welcome worker to send an email" do | ||
| expect(UserWelcomeMailerWorker).to_not receive(:perform_async) | ||
| it "should not send a confirmation instructions email" do | ||
| expect_any_instance_of(User).to_not receive(:send_confirmation_instructions) |
There was a problem hiding this comment.
RSpec/AnyInstance: Avoid stubbing using expect_any_instance_of.
RSpec/NotToNot: Prefer not_to over to_not.
| @@ -212,8 +212,8 @@ | |||
| end | |||
| end | |||
|
|
|||
| it "should not queue a welcome worker to send an email" do | |||
| expect(UserWelcomeMailerWorker).to_not receive(:perform_async) | |||
| it "should not send a confirmation instructions email" do | |||
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
RSpec/ExampleWording: Do not use should when describing your tests.
| it "should queue a welcome worker to send an email" do | ||
| expect(UserWelcomeMailerWorker).to receive(:perform_async) | ||
| it "should send a confirmation instructions email" do | ||
| expect_any_instance_of(User).to receive(:send_confirmation_instructions).once |
There was a problem hiding this comment.
RSpec/AnyInstance: Avoid stubbing using expect_any_instance_of.
| @@ -149,8 +149,8 @@ | |||
| post :create, params: { user: user_attributes } | |||
| end | |||
|
|
|||
| it "should queue a welcome worker to send an email" do | |||
| expect(UserWelcomeMailerWorker).to receive(:perform_async) | |||
| it "should send a confirmation instructions email" do | |||
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
RSpec/ExampleWording: Do not use should when describing your tests.
| @@ -72,8 +72,9 @@ | |||
| expect(response.status).to eq(200) | |||
| end | |||
|
|
|||
| it "should not send an email to the account email address" do | |||
| expect(ActionMailer::Base.deliveries).to be_empty | |||
| it 'should send confirmation instructions to the account email address' do | |||
There was a problem hiding this comment.
RSpec/MultipleExpectations: Example has too many expectations [2/1].
RSpec/ExampleWording: Do not use should when describing your tests.
|
I apologize for the Hound, but these are some deep pages that are written in the olde ways. Didn't wanna rock the boat too much and make this even more annoying to review. Most all of it is syntax or "probably don't write specs this way" when all the other specs are written that way. Please lmk if I missed anything significant, though. |
There was a problem hiding this comment.
Looks fine to me.
Some questions that you might know the answer to or answers come up through testing:
- If a user loses their confirmation email, (eg. never confirmed after a long time and now cannot post to talk because they are unconfirmed or another eg. If a person changes their email because they an created account on an email address they lost access to), is there a way to resend confirmation email?
- How long are
confirmation_tokens good for? ^ related to above
There's a form at panoptes.zooniverse.org/users/confirmation, but no one uses those. One of the front end UX things we might need is a form/button pointed at the appropriate devise route in the user settings area to resend the email.
If they confirmed with us once, that's good enough for me. No re-confirmation necessary.
Forever, they don't appear to change or expire. |
Adds the Confirmable module for Devise along with the requisite database attributes. A "please confirm your email address" email is now sent to the user on account creation. That email is using the default template (located here) and can be updated if desired.
When a user creates a new account, the front end should inform them that they will receive a confirmation email (this currently happens on Panoptes' own reg page, but no one uses it). The link in that email will point to Panoptes directly so that the API can process the user's confirmation token. That "thank you for confirming" event will likely have to be updated to redirect to wherever makes the most sense since that form currently just refreshes the panoptes API home page. Right now, successful I am redirecting successful confirmations to the zooniverse.org (production) home page. Is there a way to ensure the registration modal opens on PFE? I can include a query param that can trigger a "thank you for confirming your email address!" welcome flash message. Is that worth integrating into PFE, or should that wait until FEM is at the site root? This is a todo and should be figured out & updated before this deploys to production.
Also, when the user is confirmed they will then receive the standard Zooniverse welcome email. For reference, that template is located here and hasn't needed to be updated in a while.
Finally: there's a rake task to backfill all existing users with a
confirmed_atvalue. This field is going to be used by Talk in order to prevent unconfirmed users from interacting with Talk beyond viewing public posts. This PR must be merged and this backfill run before the Talk PR merges.This is gonna need a bit of hands on testing on staging, so once this and the Talk PR merge, that'll be the focus till production can be deployed safely.
Review checklist
apiary.apibfile?